docs(roadmap): Phase 02a packet 0 — kickoff (breakdown + glossary) - #4
Conversation
Phase 02a is too wide for a single PR; Phase 01 was shipped as eight sequential packets and the same cadence applies here. Packet 0 fixes the order before any code lands. This commit: - Prepends a Status + Packets block to phase-02a-kernel-tenancy.md defining P02a-0..P02a-10 with one-paragraph summaries and explicit dependency arrows (decisions → SharedKernel → cross-cutting → API conventions → Dapr/APISIX → tenancy schema → resolution+isolation → customization → audit+entitlement → arch-tests-catalogue green). - Adds a "Roadmap & Delivery" glossary group with canonical definitions for Phase, Packet, and Kickoff Packet — the latter spelled out so the pattern is reusable for future wide phases. - Updates README status line to point at the 11-packet breakdown. No code, no ADR state changes. Packet 1 (foundation decisions — ADR-0023 strongly-typed IDs, ADR-0024 API versioning, ADR-0028 audit partition) unblocks Packets 2+. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses two independent review-agent passes on Packet 0. Major: - glossary: "Pase-PacketIndex" → "P<PhaseId>-<PacketIndex>" (typo poisoning every downstream citation; the term had zero usages so the wrong form would have become canonical). - glossary "Phase" entry: removed broken inline link to roadmap/README.md#phase-exit-decision (that anchor only exists in per-phase docs, not the roadmap index); rephrased to cite the `## Phase Exit Decision` heading shape directly. - Packet 10: disambiguated "Packet 1's ModuleDependencyTests TODO" — the TODO lives in the Phase 01 Packet 1 backend skeleton, not the decisions-only Phase 02a Packet 1. Added a direct file+line link. Minor: - glossary: new "Roadmap & Delivery" group used `| Term | Meaning |`; the 27 preceding groups all use `| Term | Definition |`. Aligned. - Packet 0 self-description: said "added under Conventions" but the commit actually adds a new "Roadmap & Delivery" group above Conventions. Corrected. - Packet 10: replaced two invented identifiers (`No_IgnoreQueryFilters_Outside_PlatformAdmin`, `Audit_Coverage_Matrix_Exists_PerModule`) with descriptive prose + TBD-in-owning-packet notes. Regrouped the whole list by introducing packet so the catalogue-vs-future-test distinction is explicit; named `Every_OrgScoped_Entity_HasOrgIdAndFilter` / `Every_TenantOwned_Command_HasAuditCoverage` (already cited in the catalogue) where applicable. - README: replaced time-fragile "(Packet 0 — Kickoff is the most recent commit)" with the declarative "Packet 0 has shipped; Packet 1 is next." - Packet 10 closing sentence: "Closes the Phase Exit Decision checklist." → "Closes the architecture-test arm ...; the remaining gates close as their owning packets ship." (the prior wording overclaimed — six other exit gates are owned by Packets 1, 4, 6, 7, 8, 9). Suggestion accepted: - Packet 7: added cross-reference to Phase 01 Packet 8's deferred application-level seed drop-in (two demo tenants + platform admin), wired through the Tenancy module DbContext. Suggestions declined (with reasoning, no code change): - Splitting Packet 3 into 3a/3b — declined; ADR-0032's 8-step pipeline ships as one coherent surface, splitting would break the `MediatR_Pipeline_Order_Matches_Canonical_Sequence` gate. - Mermaid dependency graph for P02a-0..P02a-10 — declined for the kickoff; the dependency story is already linear and prose-encoded. Reconsider if a packet branches or runs in parallel. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Sorry @cemililik, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR updates project documentation to establish roadmap terminology and communicate Phase 02a's packet-based delivery plan. A glossary section defines Phase, Packet, and Kickoff Packet concepts; Phase 02a roadmap details a ten-packet breakdown with specific deliverables and test identifiers; and the main README updates to reference the per-packet structure and phase transition sequence. ChangesRoadmap Documentation Updates
🎯 1 (Trivial) | ⏱️ ~3 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request initiates Phase 02a (Platform Kernel + Multi-Tenancy) by shipping 'Packet 0 — Kickoff'. Key changes include updating the README with the current phase status, adding roadmap-related definitions to the glossary, and providing a detailed 11-packet execution plan in the Phase 02a roadmap document. Feedback focuses on improving documentation readability by rephrasing awkward wording regarding schema tables and standardizing the formatting of architecture test lists using nested bullet points for better consistency.
| > `LearnStack.Modules.Customization` with `TenantContentType`, | ||
| > `TenantPageBlock`, `TenantLessonItemType`, `TenantLevelTaxonomy`, | ||
| > `TenantScoringRule`, `TenantCompletionRule`, `TenantCustomFieldDef`, | ||
| > `TenantTemplateLibrary` aggregates + their schemas tables (per | ||
| > [ADR-0018](../decisions/0018-tenant-driven-customization-model.md)). |
There was a problem hiding this comment.
The phrase aggregates + their schemas tables is slightly awkward. For improved readability, consider rephrasing to aggregates and their schema tables.
| > `LearnStack.Modules.Customization` with `TenantContentType`, | |
| > `TenantPageBlock`, `TenantLessonItemType`, `TenantLevelTaxonomy`, | |
| > `TenantScoringRule`, `TenantCompletionRule`, `TenantCustomFieldDef`, | |
| > `TenantTemplateLibrary` aggregates + their schemas tables (per | |
| > [ADR-0018](../decisions/0018-tenant-driven-customization-model.md)). | |
| > `LearnStack.Modules.Customization` with `TenantContentType`, | |
| > `TenantPageBlock`, `TenantLessonItemType`, `TenantLevelTaxonomy`, | |
| > `TenantScoringRule`, `TenantCompletionRule`, `TenantCustomFieldDef`, | |
| > `TenantTemplateLibrary` aggregates and their schema tables (per | |
| > [ADR-0018](../decisions/0018-tenant-driven-customization-model.md)). |
There was a problem hiding this comment.
Applied in 6e14ff0. Also fixed the parallel phrasing in the pre-existing Scope section (line 396) for consistency — both halves of the doc now say "schema tables".
| > - From the cross-cutting ADR-0032 batch already in the catalogue | ||
| > (introduced by Packet 3): | ||
| > `IExceptionHandler_Registered_AtStartup`, | ||
| > `MediatR_Pipeline_Order_Matches_Canonical_Sequence`, | ||
| > `ValidationBehavior_DoesNotThrow_ValidationException`, | ||
| > `Domain_Methods_Do_Not_Throw_For_Expected_Cases` (Roslyn analyzer | ||
| > report), `Adapters_Wrap_Provider_Exceptions`, | ||
| > `Modules_Do_Not_Reference_Sentry_SDK_Directly`, | ||
| > `Logging_Goes_Through_Microsoft_Extensions_Logging`, | ||
| > `OTel_Pipeline_Includes_TenantContextSpanProcessor`, | ||
| > `TenantContextSpanProcessor_DoesNotThrow_When_Context_Missing`. | ||
| > - From the module-dependency arm (introduced by Packet 2 / closed by | ||
| > this packet): the Application + Infrastructure full matrix | ||
| > extending the existing Phase-01 Packet 1 `ModuleDependencyTests` | ||
| > TODO at [backend/tests/LearnStack.Tests.Architecture/ModuleDependencyTests.cs:17-21](../../backend/tests/LearnStack.Tests.Architecture/ModuleDependencyTests.cs#L17-L21); | ||
| > plus `LearnStack_Modules_DoNotReference_Hub`, | ||
| > `Modules_Do_Not_Inject_Valkey_Directly`, | ||
| > `Modules_Do_Not_Read_Entitlement_Cache_Directly`, | ||
| > `Modules_Do_Not_Write_AuditLog_Directly`, | ||
| > `Modules_Do_Not_Reference_DeploymentMode`, | ||
| > `Core_Modules_HaveNo_DomainSpecific_Names`, | ||
| > `No_Source_Folder_Named_Verticals` (already green from Phase 01). | ||
| > - From the tenancy + isolation arm (introduced by Packet 7 — canonical | ||
| > identifiers land in the catalogue when the tests do): | ||
| > `Every_OrgScoped_Entity_HasOrgIdAndFilter`; an | ||
| > `Every_TenantOwned_Entity_HasFilterAndRlsPolicy`-shaped pair for the | ||
| > tenant dimension (final name TBD in Packet 7's catalogue entry); | ||
| > "no `IgnoreQueryFilters()` outside the platform-admin scope" rule | ||
| > (final identifier TBD in Packet 7's catalogue entry). | ||
| > - From the audit arm (introduced by Packet 9): | ||
| > `AuditEntry_Inherits_Entity_Not_AuditableEntity`; | ||
| > `Every_TenantOwned_Command_HasAuditCoverage`; "audit-coverage matrix | ||
| > file exists per module" rule (final identifier TBD in Packet 9's | ||
| > catalogue entry). | ||
| > - From the Dapr arm (introduced by Packet 5): | ||
| > `Dapr_PubSub_TopicNames_FollowConvention`. |
There was a problem hiding this comment.
The lists of architecture tests under Packet 10 have inconsistent formatting (some are comma-separated text blocks, others are semicolon-separated lists). Using nested bullet points for all lists would make them more consistent and easier to read.
> - From the cross-cutting ADR-0032 batch already in the catalogue
> (introduced by Packet 3):
> - `IExceptionHandler_Registered_AtStartup`
> - `MediatR_Pipeline_Order_Matches_Canonical_Sequence`
> - `ValidationBehavior_DoesNotThrow_ValidationException`
> - `Domain_Methods_Do_Not_Throw_For_Expected_Cases` (Roslyn analyzer report)
> - `Adapters_Wrap_Provider_Exceptions`
> - `Modules_Do_Not_Reference_Sentry_SDK_Directly`
> - `Logging_Goes_Through_Microsoft_Extensions_Logging`
> - `OTel_Pipeline_Includes_TenantContextSpanProcessor`
> - `TenantContextSpanProcessor_DoesNotThrow_When_Context_Missing`
> - From the module-dependency arm (introduced by Packet 2 / closed by
> this packet):
> - The Application + Infrastructure full matrix extending the existing Phase-01 Packet 1 `ModuleDependencyTests` TODO at [backend/tests/LearnStack.Tests.Architecture/ModuleDependencyTests.cs:17-21](../../backend/tests/LearnStack.Tests.Architecture/ModuleDependencyTests.cs#L17-L21)
> - `LearnStack_Modules_DoNotReference_Hub`
> - `Modules_Do_Not_Inject_Valkey_Directly`
> - `Modules_Do_Not_Read_Entitlement_Cache_Directly`
> - `Modules_Do_Not_Write_AuditLog_Directly`
> - `Modules_Do_Not_Reference_DeploymentMode`
> - `Core_Modules_HaveNo_DomainSpecific_Names`
> - `No_Source_Folder_Named_Verticals` (already green from Phase 01)
> - From the tenancy + isolation arm (introduced by Packet 7 — canonical
> identifiers land in the catalogue when the tests do):
> - `Every_OrgScoped_Entity_HasOrgIdAndFilter`
> - An `Every_TenantOwned_Entity_HasFilterAndRlsPolicy`-shaped pair for the tenant dimension (final name TBD in Packet 7's catalogue entry)
> - "no `IgnoreQueryFilters()` outside the platform-admin scope" rule (final identifier TBD in Packet 7's catalogue entry)
> - From the audit arm (introduced by Packet 9):
> - `AuditEntry_Inherits_Entity_Not_AuditableEntity`
> - `Every_TenantOwned_Command_HasAuditCoverage`
> - "audit-coverage matrix file exists per module" rule (final identifier TBD in Packet 9's catalogue entry)
> - From the Dapr arm (introduced by Packet 5):
> - `Dapr_PubSub_TopicNames_FollowConvention`There was a problem hiding this comment.
Applied in 6e14ff0. Converted all five "introducing packet" groups to nested bullets — each test identifier on its own line, descriptive rules ("audit-coverage matrix file exists per module", etc.) sit as full clauses alongside. The deep ModuleDependencyTests.cs#L17-L21 link is preserved.
Addresses the two inline review comments from gemini-code-assist[bot] on PR #4. - Packet 8 / Scope: "aggregates + their schemas tables" → "aggregates and their schema tables" in both the Status block (line 123, the comment site) and the pre-existing Scope section (line 396, parallel phrasing of the same artifact — fixed for consistency so the two descriptions don't diverge). - Packet 10: converted each of the five "introducing packet" groups to nested bullet points. The prior shape mixed comma-separated identifier lists with semicolon-separated "named + descriptive" mixtures because inline punctuation couldn't carry both. Nested bullets let each identifier stand on its own line; descriptive rules ("audit-coverage matrix file exists per module", etc.) sit as full clauses alongside. Deep file link to `ModuleDependencyTests.cs#L17-L21` preserved. Diff is purely whitespace + grammar; no semantic change to the packet plan, no test names added or removed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Phase,Packet,Kickoff Packet— fills a single-source-of-truth gap (the term "packet" was used freely in Phase 01 commit history but undefined in the glossary).Docs-only. No code, no ADR state changes. Packet 1 moves ADR-0023 (strongly-typed ID source generator), ADR-0024 (API versioning policy), and ADR-0028 (
audit_logpartition management) from Open Drafts to Accepted; Packet 2+ then ship the Shared Kernel core and downstream foundation surface on top.Why a no-code kickoff packet
Phase 02a's scope (Shared Kernel + Dapr + APISIX + Audit + Tenancy schema + Tenant Customization + defense-in-depth isolation + cross-cutting per ADR-0032 + the architecture-test catalogue) is too wide for one PR. The kickoff packet fixes the slicing order before code lands so each subsequent PR is independently reviewable. The pattern is reusable for any future wide phase — Phase 01 didn't need it (packets fell out from the existing scaffold targets); Phase 02a does.
Commits
e929112— docs(roadmap): Phase 02a packet 0 — kickoff (breakdown + glossary)bb8decc— fix(roadmap): P02a-0 review — 3 Major + 5 Minor + 1 SuggestionSelf-review surfaced 3 Major + 5 Minor + 1 Suggestion findings on the first commit; all addressed in the fix commit (full breakdown in its body). Highlights: replaced a
Pase-PacketIndextypo in the glossary canonical form, removed a brokenroadmap/README.md#phase-exit-decisionanchor, regrouped Packet 10's architecture-test list by introducing packet so the catalogue-vs-future distinction is explicit.Test plan
dotnet build— N/A (docs-only diff; lokal SDK 9.0.109 ≠ pinned 10.0.100, CI 10.0.100 runs the actual build).backend/tests/.../ModuleDependencyTests.cs#L17-L21deep link).docs/analysis/residual scan on changed files — 0 hits.| Term | Definition |matches the 27 preceding groups.Packet 10name either already exists in 21-architecture-tests-catalogue.md (the ADR-0032 batch) or is flagged "(final identifier TBD in Packet N's catalogue entry)".Notes for the reviewer
scripts/seed.shplaceholder retires when the Tenancy moduleDbContextlands.🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation